Skip to content

feat(terminal): newline chord as registry data, plus a Key tester in Settings - #522

Merged
Ark0N merged 2 commits into
Ark0N:masterfrom
opticon454:fix/newline-sequence-capability
Oct 4, 2026
Merged

feat(terminal): newline chord as registry data, plus a Key tester in Settings#522
Ark0N merged 2 commits into
Ark0N:masterfrom
opticon454:fix/newline-sequence-capability

Conversation

@opticon454

@opticon454 opticon454 commented Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What

Moves "which bytes does Shift+Enter type into this CLI's pane" out of the send-key route and into the CLI registry, and adds a Key tester to Settings.

  • capabilities.newline ('line-feed' | 'esc-enter', absent = line feed). POST /api/sessions/:id/send-key reads it for S-Enter instead of hardcoding 0x0a; C-Enter is always a line feed. No stock CLI declares it, so behaviour is unchanged for every CLI today: it is the place to put a CLI whose composer ignores a bare line feed (or a user clis.json override) instead of a mode === 'x' check in the route. It is an enum, not a byte string, so config never carries bytes that get typed into a pane.
  • Key tester (Settings → Terminal & Input): click the box, press keys, see what this browser reports for keydown/keypress/keyup (key, code, modifiers, charCode). Read-only, sends nothing to a session. It does not preventDefault on keydown (that would suppress the keypress that mattered in fix(terminal): Shift+Enter no longer submits after inserting a newline #520), and keys pressed in it do not trigger app shortcuts: the global shortcut dispatcher skips events aimed at a [data-raw-keys] element.

Why

Shift+Enter handling has three layers (browser handler, send-key bytes, the CLI's own composer). Making the middle one data keeps the registry's no-id-branching rule true, and the Key tester makes the next "Shift+Enter does X on my device" report diagnosable in seconds. Independent of #520 (the keypress fix); they touch different files apart from the test list.

Tests

  • test/cli-newline-capability.test.ts: no stock CLI declares a chord; schema accepts the two values and rejects free-form byte strings; optional.
  • test/routes/session-routes.test.ts: bytes sent to tmux per mode (line feed by default, including codex; ESC CR only for a CLI that declares it, exercised via a clis.json override; Ctrl+Enter always a line feed; unknown mode falls back to a line feed).
  • test/key-tester.browser.test.ts (real Chromium): Shift+Enter shows the keypress with charCode=13, Ctrl+Enter shows none, the field never types, and Ctrl+W / Ctrl+L / Escape / Alt+1 / Ctrl+K pressed in it fire no app shortcut and leave Settings open (verified to fail without the dispatcher guard), while Escape elsewhere still reaches closeAllPanels.
  • Full CI gate: typecheck, lint, format, public assets, catalogue and 8525 tests pass.

🤖 Generated with Claude Code

@opticon454 @claude
feat(terminal): newline chord as registry data, plus a Key tester in …
f39c66e 
…Settings

capabilities.newline replaces choosing the Shift+Enter bytes in the send-key
route. Key tester shows the keydown/keypress/keyup a browser reports.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@Ark0N

Ark0N commented Oct 2, 2026

Copy link
Copy Markdown
Owner

Thanks @opticon454, this moves the Shift+Enter byte choice of the send-key route into the CLI registry as an enum and adds a Key tester to Settings. The registry half is clean: enum-only config, resolved at call time, no id branching, and the route tests pin the bytes per mode. The full gate is green here too (8523 tests).

A few things before it goes in:

  1. Key tester lets app shortcuts fire (src/web/public/settings-ui.js:1122, root cause src/web/public/app.js:1244). The shortcut dispatcher is a document capture-phase keydown listener with no focus-target check, so it runs before the tester's handler. With focus in #keyTesterInput in Chromium, Ctrl+W called killActiveSession() (in the app that closes the active session and kills its tmux, no confirm), Ctrl+L cleared the terminal, and Escape closed Settings. Alt+1..9, Ctrl+Tab and the other registry chords go through the same path. On macOS and in an installed PWA window Ctrl+W reaches the page, so probing "what does Ctrl+W send" loses the session, while the row says nothing is sent to a session. Please make the dispatcher return early for events aimed at the tester, as its first statement (for example if (e.target && e.target.id === 'keyTesterInput') return;, or a data-raw-keys marker checked with closest() so the shortcut-rebind input can opt in later). Keep not calling preventDefault, so keypress still fires. Then extend test/key-tester.browser.test.ts: stub app.killActiveSession and app.clearTerminal, press Ctrl+W, Ctrl+L and Escape in the field, and assert nothing was called and Settings is still open.

  2. Codex switch to ESC CR (src/config/cli-registry/stock.ts:623). The bytes are typed by tmux on the server, so the browser's OS cannot change what Codex reads. I tested codex-cli 0.147.0 in a tmux 3.4 pane: send-keys -H 0a inserts a newline, and 1b 0d does too. fix(codex): make Shift+Enter insert a newline in Windows terminals #495's symptom matches the keypress leak you fixed in fix(terminal): Shift+Enter no longer submits after inserting a newline #520. Unless you have a Codex version or setup where the line feed really fails, please leave Codex on the line feed (drop the stock.ts line and have test/cli-newline-capability.test.ts assert that no stock CLI declares a chord). If you do have one, keep esc-enter and change the comment, docs/cli-registry.md:123 and the changeset to describe that case.

  3. Port (test/key-tester.browser.test.ts:6): 3197 is already used by test/base-path-server.test.ts:11. Please pick an unused one.

  4. Ctrl+Enter test (test/key-tester.browser.test.ts:40): the name says "without a keypress" but the test does not check it. Chromium emits none, so expect(text).not.toMatch(/keypress/) passes.

Smaller things, fine in the same push:

  • src/web/routes/session-routes.ts:2096 and docs/architecture-invariants.md (lines 194 and 783) still say send-key always injects 0x0a. Point them at capabilities.newline if Codex keeps esc-enter.
  • src/web/public/index.html:1834 uses class="set-select" where the other settings text inputs use set-input, and line 1825 has trailing whitespace.
  • config/test-suites.ts:34 conflicts with fix(terminal): Shift+Enter no longer submits after inserting a newline #520 (both PRs insert at the same line). I plan to merge fix(terminal): Shift+Enter no longer submits after inserting a newline #520 first, so this one will need a rebase that keeps both lines. After that, the "used to submit" wording in settings-ui.js:1120 and the test comment at line 35 will be accurate.

Once 1 to 4 are addressed I'll merge it.

@opticon454 @claude
fix(terminal): address review: Key tester isolates shortcuts, Codex s…
2cf3752 
…tays on line feed

- app.js: the shortcut dispatcher returns early for events aimed at a data-raw-keys
  field, so Ctrl+W / Ctrl+L / Escape / Alt+1 / Ctrl+K pressed in the Key tester no
  longer kill the session, clear the terminal or close Settings
- stock.ts: drop Codex's esc-enter (a line feed works); no stock CLI declares a chord.
  The esc-enter path is tested through a clis.json override
- tests: unused port (3194), Ctrl+Enter asserts no keypress, shortcut-isolation test
  (verified to fail without the guard)
- docs/comments point at capabilities.newline; set-input class, trailing whitespace

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@opticon454

opticon454 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, all four are fixed in the push above, plus the smaller items. Full gate on the branch: typecheck, lint, format, public assets, catalogue and 8525 tests pass.

  1. Key tester vs app shortcuts. The dispatcher in app.js now returns early, as its first statement, for events aimed at a [data-raw-keys] element (matched with closest()), and the tester input carries that attribute. keydown is still not preventDefaulted, so keypress still fires. test/key-tester.browser.test.ts now stubs killActiveSession, clearTerminal, openCommandPalette and closeAllPanels, presses Ctrl+W, Ctrl+L, Escape, Alt+1 and Ctrl+K in the field, asserts none of them was called, that each chord still shows up in the tester, and that Settings is still open. I also ran it with the guard removed to check the test means something: without the guard killActiveSession and two others fire and the test fails. A second test pins that the guard is scoped, so Escape outside the field still reaches closeAllPanels.
  2. Codex. Dropped. stock.ts no longer declares a chord and test/cli-newline-capability.test.ts asserts no stock CLI does. You are right that tmux types the bytes on the server; I had attributed fix(codex): make Shift+Enter insert a newline in Windows terminals #495 to a byte problem when it matches the keypress leak from fix(terminal): Shift+Enter no longer submits after inserting a newline #520. The esc-enter path is still covered, through a clis.json override of codex in test/routes/session-routes.test.ts (Esc+Enter for Shift+Enter on that CLI only, Ctrl+Enter always a line feed). docs/cli-registry.md and the changeset now describe the field as available for a CLI that needs it, not as something Codex uses.
  3. Port. 3194 (3197 is base-path-server.test.ts; I also checked against the other ports in the tree).
  4. Ctrl+Enter test. Now asserts not.toMatch(/keypress/).

Smaller items: the send-key route comment and the two architecture-invariants.md mentions point at capabilities.newline; the input uses set-input; the trailing whitespace is gone.

On the config/test-suites.ts conflict with #520: agreed that #520 goes first. I have not rebased yet since #520 is not merged; once it is, I will rebase onto master keeping both lines and push.

@opticon454 opticon454 mentioned this pull request Oct 3, 2026
Merged
@opticon454

opticon454 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Adding to the config/test-suites.ts note above: I checked my open PRs against each other with trial merges, and #522 overlaps the others too. Textual conflicts only (adjacent insertions), no behavioural interaction:

Nothing to do on this PR now. Once #520 merges (and for whichever of #521/#523 lands before this one) I will rebase onto master keeping both sides, re-run the full gate and push.

@opticon454 opticon454 mentioned this pull request Oct 3, 2026
Merged
@Ark0N
Ark0N merged commit c197e93 into Ark0N:master Oct 4, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Oct 4, 2026
@claude
fix(terminal): drive the shipped Shift+Enter handlers and the Key tes…
52267f8 
…ter cap in their browser tests (#520, #522 review)

- Minor: the Key tester "14 lines" test never pressed a key into the
  tester (the previous test blurred it, so the presses landed on <body>
  and the cap was never exercised). It now refocuses the field, asserts
  the focus, clears the log, checks 2 presses accumulate to 6 lines, then
  4 presses of another key cap the log at exactly 14 with the oldest 4
  lines evicted in order, and the readonly field stays empty. Verified to
  fail with the cap changed to 20.
- Nit: the split-pane invariant implied Ctrl+Enter could use the CLI's
  declared newline chord. Reworded after checking the send-key route:
  Ctrl+Enter is always a real 0x0a, Shift+Enter is the declared
  capabilities.newline chord (0x0a unless the CLI declares another), sent
  on keydown only. The same imprecision in the auto-named sessions
  paragraph is corrected too.
- Nit: docs/wiki/Settings-Reference.md now lists the Key tester row in
  the Terminal & Input table.

- Nit: test/shift-enter-keypress.browser.test.ts exercised a hand-copied
  predicate named `shipped`. It now loads the real app from a real
  WebServer and presses real keys into the handlers terminal-ui.js
  (app.terminal, recording the real _sendInputAsync send path) and
  terminal-split.js (a real SplitTerminalPane) attach, recording the
  send-key POSTs through a fetch wrapper. It asserts no \r reaches either
  send path for Shift/Ctrl+Enter, exactly one send-key per press for the
  right session, and that Enter and Alt+Enter are untouched. The old
  keydown-only gate stays as a labelled reproduction of xterm's keypress
  behaviour on a bare Terminal. Verified to fail on both panes with the
  gate narrowed back to keydown.
- Nit: the keypress trap is now written down beside the other key-gate
  rules (Command palette and shortcut registry): xterm runs the custom
  handler for keydown, keypress and keyup and drops only Ctrl/Alt/Meta
  keypresses, so a gate on a chord that can carry Shift alone must
  swallow every event type. The smart-copy keydown-only rule points at it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Ark0N pushed a commit that referenced this pull request Oct 4, 2026
@claude
chore: changeset for the 1.34.0 landing (#499, #514, #515, #517, #519, …
6aecc3b 
…#520, #521, #522, #523, #524, #530, #531)

One consolidated minor changeset with the Thanks block first; the four contributor changesets (#520, #521, #522, #523) are folded into it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ark0N

Ark0N commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Merged and shipped in 1.34.0. Thanks @opticon454! The data-raw-keys guard on the shortcut dispatcher is the right shape: the tester is safe now, and the shortcut-rebind input can opt into the same marker later. Dropping the codex chord once the keypress leak from #520 explained #495 was also the right call.

I applied the remaining review items while landing (52267f8, docs and tests only):

  • The 14-line test now really drives keys into the tester. It checks the log accumulates and then caps at exactly 14, and it fails if the cap changes.
  • The split-pane invariant now says Ctrl+Enter is always a real 0x0a and only Shift+Enter uses the declared chord. The same slip in the auto-named sessions paragraph is fixed too.
  • The Key tester has a row in docs/wiki/Settings-Reference.md.

The config/test-suites.ts overlaps with #520 and #523 were resolved during the landing, so there is nothing left for you to rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@opticon454 @Ark0N